perf(bigtable): build the entry proto once in createResource - #14017
Open
laughingman7743 wants to merge 1 commit into
Open
perf(bigtable): build the entry proto once in createResource#14017laughingman7743 wants to merge 1 commit into
laughingman7743 wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request optimizes the 'createResource' method in 'MutateRowsBatchingDescriptor' by calling 'element.toProto()' only once instead of twice, preventing redundant proto constructions. It also adds corresponding unit tests to verify the resource creation behavior. There are no review comments, so I have no feedback to provide.
MutateRowsBatchingDescriptor.createResource() built the same MutateRowsRequest.Entry twice for every element added to a bulk-mutation batcher: once through countBytes() to read the serialized size, and once directly to read the mutation count. RowMutationEntry.toProto() memoizes nothing, so the second construction was pure overhead, and BatcherImpl.add() calls createResource for every accepted element. Build the proto once and read both values from it. Behaviour-preserving. Measured, this halves the bytes allocated per element. MutateRowsBatchingDescriptorTest had no coverage of createResource or createEmptyResource; both are added.
laughingman7743
force-pushed
the
perf-bigtable-createresource-single-toproto
branch
from
August 8, 2026 13:01
b844208 to
59d1d1a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #14016.
The defect
createResource()builds the sameMutateRowsRequest.Entrytwice for every element added to abulk-mutation batcher — once through
countBytes()to read the serialized size, once directly toread the mutation count:
RowMutationEntry.toProto()memoizes nothing — it builds a fresh proto per call — so the secondconstruction is pure overhead.
BatcherImpl.add()callscreateResourcefor every acceptedelement (
BatcherImpl.java:232, gax 2.82.0), so this is per mutation, not per batch.The change
Build the proto once and read both values from it. Behaviour-preserving: the two values come from
an identical proto.
countBytesis deliberately left alone — it remains part of theBatchingDescriptorcontract andis still called from
TracedBatchingCallableand from the interface's own defaultcreateResource.What it saves
One
toProto()construction per element. Measured on OpenJDK 21.0.5 (aarch64,-Xmx4g, defaultcollector) against
google-cloud-bigtable2.80.0 andgax2.82.0, entries pre-built into a pool,allocation sampled over the same loop as the timing:
I am not claiming a wall-clock figure. The end-to-end A/B on my machine ranged from -73% to
+23% for the same shape across JVM forks — noise, not signal — and I would rather say so than quote
the flattering end of it. The allocation halving above was exact in every fork and every shape, and
one
toProto()construction measured in isolation (three forks, its own arm) costs 27.1 ns at onecell rising to 2433.1 ns at a thousand. If you have a JMH harness you trust, its number should be
preferred to mine.
The saving is the construction only, not a serialized-size walk:
Mutation.addMutation()alreadycalls
getSerializedSize()per cell, and protobuf memoizes size per message instance, so theshared child protos are warm before either call. The allocation grows at roughly 16 B per mutation
— two eight-byte references — which is what identifies it as reference copying rather than payload
copying.
Verification
MutateRowsBatchingDescriptorTesthad no coverage ofcreateResourceorcreateEmptyResourceatall; both are added, and the whole class passes (9 tests, was 7).
These are characterization tests, and they do not discriminate the change — the fix is
behaviour-preserving, so they pass against the unpatched code too, which I checked rather than
assumed. What they do is pin behaviour that was previously unpinned. That they can fail at all was
verified by mutation:
createResourcecreateResourceTestandcreateEmptyResourceTestfail1->0createResourceTestfailsBoth restored afterwards.
google-java-formatreports both changed files compliant.